-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add observability to Bedrock Titan Embedding model #3014
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add observability to Bedrock Titan Embedding model #3014
Conversation
0ff770f
to
95ba402
Compare
Thanks. A few things to clean up in the PR.
adding
and changing the bean def to be
Also, you can use |
0cd6a52
to
ed41e60
Compare
Signed-off-by: PSriVarshan <[email protected]>
Signed-off-by: PSriVarshan <[email protected]>
Signed-off-by: PSriVarshan <[email protected]>
Signed-off-by: PSriVarshan <[email protected]>
Signed-off-by: PSriVarshan <[email protected]>
Signed-off-by: PSriVarshan <[email protected]>
ed41e60
to
b127661
Compare
I think now the branch is fully ready and optimal. Thanks a ton for the guidance @markpollack. Waiting for your further guidance. |
@PSriVarshan Thanks for the PR and following up with the changes. |
<dependency> | ||
<groupId>io.micrometer</groupId> | ||
<artifactId>micrometer-observation</artifactId> | ||
<version>1.15.0-RC1</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need an explicit version here.
return new TitanEmbeddingBedrockApi(properties.getModel(), credentialsProvider, regionProvider.getRegion(), | ||
objectMapper, awsProperties.getTimeout()); | ||
} | ||
|
||
@Bean | ||
@ConditionalOnMissingBean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to other auto configurations, we don't provide the default ObservationRegistry instead provide NOOP registry when none provided. Will update the PR when merging.
Squashed, addressed the review comments and pushed the changes via a36c709 |
Description
This PR introduces observability to the Bedrock Titan Embedding Model by adding Micrometer
Observation
around the embedding API call. This ensures consistent metrics and tracing support in accordance with the Spring AI observability pattern.ObservationRegistry
.model
,input_type
, andinput_length
.Changes
BedrockTitanEmbeddingModel.java
to wrap the API call inObservation.createNotStarted().observe(...)
Related Issue
Fixes #2983
Checklist
ObservationRegistry
Do let me know if there are any further modifications required within the PR